Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update Heat Tutorials, fix typos. #1238

Merged
merged 6 commits into from
Oct 16, 2023
Merged

Update Heat Tutorials, fix typos. #1238

merged 6 commits into from
Oct 16, 2023

Conversation

LScheib
Copy link
Collaborator

@LScheib LScheib commented Oct 10, 2023 •
edited by ClaudiaComito
Loading

Due Diligence

  • General:
    • base branch must be main for new features, latest release branch (e.g. release/1.3.x) for bug fixes
    • title of the PR is suitable to appear in the Release Notes
  • Implementation:
    • unit tests: all split configurations tested
    • unit tests: multiple dtypes tested
    • documentation updated where needed

Description

Issue resolved: #1235
To get started with Heat I went through the documentation and particularly the provided tutorials. While doing so, I noticed a few minor linguistic mistakes and typos in code snippets, which I correct here. Also I try to slightly improve the uniformity of the given code examples.

Does this change modify the behaviour of other functions? If so, which?

no

@ghost
Copy link

ghost commented Oct 10, 2023 •
edited by ghost
Loading

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@github-actions
Copy link
Contributor

Thank you for the PR!

@codecov
Copy link

codecov bot commented Oct 10, 2023 •
edited
Loading

Codecov Report

Merging #1238 (368646e) into main (5beb254) will increase coverage by 0.57%.
Report is 1 commits behind head on main.
The diff coverage is n/a.

❗ Current head 368646e differs from pull request most recent head 6468616. Consider uploading reports for the commit 6468616 to get more accurate results

@@            Coverage Diff             @@
##             main    #1238      +/-   ##
==========================================
+ Coverage   91.75%   92.32%   +0.57%     
==========================================
  Files          77       77              
  Lines       11080    11056      -24     
==========================================
+ Hits        10166    10207      +41     
+ Misses        914      849      -65
Flag Coverage Δ
unit 92.32% <ø> (+0.57%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@LScheib LScheib requested a review from mrfh92 October 11, 2023 11:24
@LScheib LScheib self-assigned this Oct 11, 2023
mrfh92
mrfh92 previously approved these changes Oct 11, 2023
View reviewed changes
Copy link
Collaborator

@mrfh92 mrfh92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me.
Thanks 👍

@mrfh92 mrfh92 marked this pull request as ready for review October 11, 2023 12:23
@mrfh92
Copy link
Collaborator

mrfh92 commented Oct 11, 2023

CI seems to get stuck at "ci/codebase Expected — Waiting for status to be reported"...

@mrfh92
Copy link
Collaborator

mrfh92 commented Oct 12, 2023

CI gets stuck again... this is strange since, e.g., #1236 did not get stuck

@mrfh92
Copy link
Collaborator

mrfh92 commented Oct 13, 2023

@mtar when youre fit again, can you have a look on this?

@mrfh92 mrfh92 added PR talk documentation Anything related to documentation labels Oct 13, 2023
@ClaudiaComito
Copy link
Contributor

I think the pipeline isn't triggering because Receive PR isn't triggered by changes in doc/ (for good reasons). We should merge anyway.

@ClaudiaComito ClaudiaComito changed the title Minor linguistic corrections in the Heat Tutorials. Update Heat Tutorials, fix typos. Oct 16, 2023
ClaudiaComito
ClaudiaComito approved these changes Oct 16, 2023
View reviewed changes
Copy link
Contributor

@ClaudiaComito ClaudiaComito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @LScheib !!

@ClaudiaComito ClaudiaComito merged commit cd887e4 into main Oct 16, 2023
5 checks passed
@mtar mtar removed the PR talk label Nov 9, 2023
@mtar mtar deleted the 1235-Minor_corrections_in_Heat_Tutorials_in_documentation branch February 28, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ClaudiaComito ClaudiaComito ClaudiaComito approved these changes

@mrfh92 mrfh92 mrfh92 left review comments

Assignees

@LScheib LScheib

Labels
documentation Anything related to documentation merge queue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Minor corrections in Heat Tutorials in documentation
4 participants
@LScheib @mrfh92 @ClaudiaComito @mtar